-
Notifications
You must be signed in to change notification settings - Fork 742
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ci] publish snapshots with every commit to main #16
Conversation
c1be77a
to
f207bfb
Compare
@Shastick could you please provide a bit of details, I am not sure I understand how it works and why we need this. Thanks a lot! |
This was meant as a draft to try out publishing to sonatype via github actions. I'll reopen when I had some time to explore this |
This would actually be pretty useful, especially while this is still under active development. Would be nice to get bugfixes and new features faster than once every few weeks with a new release. |
@langchain4j should I take a look at this again? I’ll likely need some credentials to push to Sonatype: You could either grant me access (I have a sonatype account that I could give you the name off), or add your credentials as secrets somewhere in the Github actions settings. |
@Shastick where can/should we host snapshots? By Sonatype you mean the same repo where we host released artefacts? |
Yes, at sonatype: if I remember correctly the process is roughly the same as for a normal release, except that:
|
Hi @Shastick, sorry for the delay. I have added |
c9d20ec
to
06b90fc
Compare
I'm giving this a try and seem to get 401's from sonatype: are the credentials still up to date? Ie, we get:
For example on this run @langchain4j could you try running a snapshot build locally (I assume it's a |
06b90fc
to
0b9a445
Compare
Hi @Shastick sorry for delay. I have set I am not sure the need for I have updated |
Maybe try using |
0b9a445
to
7ff1be6
Compare
@langchain4j Looks like we make it one step further, but now:
I'll try to check later, but possibly you have something in your settings about |
@Shastick I am getting the same error locally. I guess it tries to deploy aggregator pom which does not contain all mandatory information like scm links, etc. But we do not publish anyway, so we could exclude this from the build. Other modules are published ok as far as I see, right? BTW could you please remind why we went with this parent/aggregator separation? |
7ff1be6
to
a641470
Compare
50f371b
to
66b656e
Compare
OSSRH_USERNAME: ${{ secrets.OSSRH_USERNAME }} | ||
OSSRH_PASSWORD: ${{ secrets.OSSRH_PASSWORD }} | ||
# These don't work with java 8. | ||
# Consider have something separate with more recent versions for the modules that don't support older versions? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might use matrix approach as we do in https://github.com/langchain4j/langchain4j/blob/main/.github/workflows/main.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed that would make sense.
What is the approach for releases? Is everything released from the older version of Java that is possible?
If you have a script or a bunch of options that are used for release they could be useful here, otherwise I can cobble something together from the existing workflow files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the approach for releases? Is everything released from the older version of Java that is possible?
Yes,
- we release with java 8 everything except
langchain4j-opensearch
,langchain4j-neo4j
andlangchain4j-code-execution-engine-graalvm-polyglot
- then
langchain4j-opensearch
with 11 - then
langchain4j-neo4j
andlangchain4j-code-execution-engine-graalvm-polyglot
with 17
I do not have any script unfortunately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have a look, I think that should be feasible in some way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One side question though: is it important that all snapshots for a release from master share the same version?
Currently it seems based on time: I assume we could have something based on the commit hash if required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I guess users should be able to use X.Y.Z-SNAPSHOT
version and maven should provide the latest snapshot, or?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, makes sense. I'll look into that once I got the rest to work
@Shastick thanks a lot! What was the problem BTW? wrong env variable names? |
Yes, it seems I got confused with the environment variables |
d1fc372
to
cb21c3d
Compare
@langchain4j did we maybe overthink things a bit? Given the |
cb21c3d
to
8b74333
Compare
Release works (see this run for example). I assume we can trust the backward compatibility (all modules specify the minimal java version to target), so we don't need to build on multiple JDKs (and we can always revisit if required) Also, it seems that the version is as expected: ie, here is the metadata for All modules can be seen here: https://s01.oss.sonatype.org/content/repositories/snapshots/dev/langchain4j/ |
8aa94ce
to
8f158cf
Compare
8f158cf
to
a6390be
Compare
@Shastick thank you so much and sorry for the late reply. Yeah seems that I indeed overthinked it and we can have a single build. Awesome! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Shastick great job, thank you!
Adds a workflow for publishing a snapshot to sonatype with each commit to main.